Implement Evaluator Versions #402
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Introduced a new `fireworks_client.py` module to centralize Fireworks SDK client creation. - Updated CLI and evaluation modules to use the new `create_fireworks_client` function instead of direct instantiation of the Fireworks class. - Enhanced handling of API key, account ID, base URL, and extra headers through environment variables. - Added tests for the new Fireworks client factory to ensure proper functionality and configuration.
- Added functionality to load environment variables from .env.dev or .env as a fallback when the auth module is imported. - Updated the API key verification process to allow explicit base URL handling, defaulting to dev.api.fireworks.ai if not provided. - Removed redundant environment variable loading code from platform_api module.
- Introduced functionality to create evaluator versions using parameters such as commit hash, entry point, and requirements. - Updated the upload endpoint call to utilize the newly created evaluator version ID instead of a hardcoded test version ID. - Added error handling for missing evaluator version ID in the response to ensure robustness during code uploads.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
update to latest once SDK is published with changes
- Implemented a try-except block to handle APIStatusError during evaluator creation. - Added logic to check for existing evaluators and retrieve the existing one if a conflict occurs (status code 409). - Enhanced logging for better traceability of evaluator creation process.
…d signature introspection, avoiding unnecessary API requests during help invocations.
…dating polling functions to target specific evaluator versions. Refactor related CLI commands and tests to accommodate these changes, ensuring clearer status messages and improved error handling.
| if "api.fireworks.ai" in provided_base: | ||
| resolved_base = provided_base | ||
| else: | ||
| resolved_base = "https://dev.api.fireworks.ai" |
There was a problem hiding this comment.
Verification endpoint defaults to dev for custom API bases
Medium Severity
The verify_api_key_and_get_account_id function now falls back to https://dev.api.fireworks.ai for the verification call when the API base URL doesn't contain "api.fireworks.ai". This causes authentication failures for users with custom endpoints (proxies, internal deployments) who have production API keys, since the verification request goes to dev which rejects production credentials. The fallback should likely use production (https://api.fireworks.ai) instead of dev, or the user's configured base URL should be respected.
| logger.warning(f"Code upload failed (evaluator created but code not uploaded): {upload_error}") | ||
| # Don't fail - evaluator is created, just code upload failed | ||
| # Return None for version_id since upload failed | ||
| return result, None |
There was a problem hiding this comment.
Tar file not cleaned up when upload fails
Medium Severity
When the upload process fails after the tar.gz file is created at tar_path, the exception handler returns (result, None) without removing the tar file. The cleanup at lines 347-349 is only reached on the success path. This leaves a {dir_name}.tar.gz file in the user's working directory after every failed upload attempt, which could be large and confusing.
Additional Locations (1)
…iable loading into local test command. Introduced functions to find and retrieve values from .env files, enhancing configuration management for Docker tests.
…iable loading into local test command. Introduced functions to find and retrieve values from .env files, enhancing configuration management for Docker tests.
…mplement-evaluator-versions
…kspace so it should include the .env file
- improving environment variable management and preventing conflicts with other .env files.
- improving environment variable management and preventing conflicts with other .env files.
…valuator-versions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
Introduces evaluator versioning and consolidates Fireworks SDK usage, while tightening environment handling and tests.
evaluation.create()now createsevaluator_versions, uploads code viaevaluator_versions.get_upload_endpoint, validates viaevaluator_versions.validate_upload, and returns(result, version_id)create_rftuploads evaluator if needed and polls specific version forACTIVEvia_poll_evaluator_version_status; removes deprecated--forcehandlingeval_protocol/fireworks_client.pyfor a unifiedcreate_fireworks_client()with support forFIREWORKS_EXTRA_HEADERS; refactors callers (evaluation,platform_api, RFT job creation) to use itload_dotenvpaths in CLI and Vercel quickstart;.authadds.env.dev/.envresolution helpers; removes implicit find-dotenv usage elsewherefireworks-aito1.0.0a22; minor .gitignore/.vscode housekeepingWritten by Cursor Bugbot for commit ab04086. This will update automatically on new commits. Configure here.